-
Notifications
You must be signed in to change notification settings - Fork 3
Doc Improvements #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: coderabbitai/utg/b5c00ce
Are you sure you want to change the base?
Doc Improvements #3
Conversation
Summary of ChangesHello @JaclynCodes, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's robustness and maintainability by expanding test coverage, standardizing core dependencies, and refining CI/CD pipelines. The changes aim to improve code quality, catch regressions earlier, and ensure a more consistent development environment across various modules and examples. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request significantly improves test coverage for several utility modules and updates dependencies across the repository. The new tests are comprehensive and cover many edge cases. I've left one comment regarding test isolation for the logging utilities to improve test robustness. The addition of TEST_ENHANCEMENTS.md is a great way to document these improvements. The dependency updates seem to be a good housekeeping measure to ensure stability.
test/unit/build-output-log.test.ts
Outdated
| it('should cache based on complete message combination', () => { | ||
| warnOnce('part1', 'part2') | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('part1', 'part2') | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('part1', 'part3') | ||
| expect(messages.length).toBe(2) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests for warnOnce are not properly isolated. warnOnce uses a module-level cache that is not reset between tests. This can lead to flaky tests that depend on execution order.
To fix this, you need to reset Jest's module cache and get a fresh instance of warnOnce for each test. This should be applied to all tests for warnOnce in this suite.
The same issue applies to the errorOnce tests. While they use require inside each test, they are missing jest.resetModules(), so they are also not correctly isolated.
it('should cache based on complete message combination', () => {
jest.resetModules()
const { warnOnce } = require('next/dist/build/output/log')
warnOnce('part1', 'part2')
expect(messages.length).toBe(1)
warnOnce('part1', 'part2')
expect(messages.length).toBe(1)
warnOnce('part1', 'part3')
expect(messages.length).toBe(2)
})…#88229) The transform output for 'use cache' uses `Object.defineProperty` and `Array.prototype.slice`. In theory, it's possible for a module to shadow those names and break it: ```ts // i want to suffer! const Array = {} const Object = {} async function cached() { "use cache" ... } ``` We should defensively make sure that our transform's output refers to the actual Object/Array builtins, not whatever's in scope for that name. We can do this by using an unresolved syntax context for the identifiers. This is what we do for `require()` elsewhere: https://github.com/vercel/next.js/blob/6805af48baf1128e6b2a9e8f1752ee5982f02c60/crates/next-custom-transforms/src/transforms/track_dynamic_imports.rs#L119-L122 Using `quote_ident!(unresolved_ctxt, "Array")` will cause SWC to rename any local redefinitions of `Array` to something that doesn't clash (which can be seen in the added snapshots).
Support multi denied_paths in `DiskFileSystem`, it's useful to reduce redundant task invalidation.
React already disallows javascript URLs. This change extends this prohibition to Next.js specific APIs that don't serialize to an href on an anchor tag. There are workarounds if need be but this is considered a bugfix because these URLs are non-routable and the affected APIs were never intended to work with these URLs. This replaces prior efforts here: vercel#64779 which attempted to make this capability optional. We decided it was just not supportable and since workarounds exist the burden of upgrading through this bugfix is acceptable. --------- Co-authored-by: Zack Tanner <[email protected]>
Somewhat surprisingly, no breaking API changes swc-project/swc@v1.15.5...v1.15.8 tracks the changes
This was caught by our React 18 tests which only run on `canary`. There was a console error about breaking `javascript:` urls in an upcoming React release but it didn't actually block them.
…el#88242) We may have many `Invalidator`s, and we're often looping over collections of them. Instead of storing a weak reference to `TurboTasksApi` and tokio's `Handle` inside of `Invalidator`, it's better for the callsite to track this. `Weak<dyn TurboTasksApi>` and `Handle` are both 16-byte fat pointers, and there's some alignment overhead, so **this reduces the size of Invalidator from 40 bytes to 4 bytes** (according to RA), and reduces the number of refcount increment/decrements we have to do when using invalidators. These changes were LLM-generated, with careful manual review and cleanup.
This ensures we properly relativize URLs in node middleware when deployed and local. Tests weren't catching this previously as none were asserting the raw Location header value due to node-fetch auto-resolving the value. Fixes: vercel#87950 Closes: NEXT-4826
### What? Remove unused code
…ercel#87318) <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # -->
### What? avoid using blocking tasks for IO work as we can't control the priority of blocking tasks. ### Why? We already control concurrency of fs access via a semaphore which helps to ensure we won't tie up all tokio threads and we are generally suspicious that moving this blocking work to blocking threads is worthwhile
### What? Previously when the trace depth reaches 150 it ignored the spans and therefore lost time. This fixes it by flattening spans instead of dropping them. This keeps the time, but still restricts the depth. Also limits depth to 80 instead of 150 which is more reasonable.
…d process (vercel#88230) ## Eliminate duplicate `loadConfig` calls during development server startup ### What? Remove duplicate `loadConfig` calls from the parent process (`next-dev.ts`) by passing `distDir` via IPC from the child process where config is already loaded. ### Why? Previously, when booting the development server: 1. The **child process** (`start-server.ts` → `router-server.ts`) would call `loadConfig` to get the full configuration for the actual server 2. The **parent process** (`next-dev.ts`) would also call `loadConfig` separately just to access `config.distDir` for telemetry and trace uploads Since these are separate processes, the in-memory config cache couldn't be shared, resulting in redundant config loading work. ### How? Extended the existing IPC messaging to include `distDir` when the child process sends `nextServerReady`: ``` Parent (next-dev.ts) Child (start-server.ts → router-server.ts) | | |-------- fork() ------------------>| |<------- nextWorkerReady ----------| |-------- nextWorkerOptions ------->| | | loadConfig() ← only called here now |<-- nextServerReady + distDir -----| | | Store distDir for telemetry/trace uploads ``` **Changes:** - **`render-server.ts`**: Added `distDir` to `ServerInitResult` type - **`router-server.ts`**: Return `config.distDir` in `initialize()` result and pass to render server opts - **`start-server.ts`**: Return `distDir` from `startServer()` and include it in IPC message - **`next-dev.ts`**: Receive `distDir` from IPC, remove both `loadConfig` calls, use `distDir` directly for telemetry/trace
…ge (vercel#88247) This updates to expand our normalized interface of handler(req, ctx) to edge runtime as well and middleware. This also applies some fixes for outputs and routes for app router. Validated against our test suite here vercel/vercel#14102 to start. Once nextjs/adapter-vercel#6 is landed we can start wiring up the full E2E test suite for validating.
## Move "Ready in X" log before config loading and eliminate duplicate `loadConfig` calls ### What? Reorder the dev server startup flow so the "Ready in X" log appears before loading the user's `next.config.js`, and eliminate duplicate `loadConfig` calls in both dev and build paths. ### Why? Previously, the "Ready in X" time included the time spent loading and evaluating the user's `next.config.js` file. This made the framework appear slower than it actually is, especially for projects with complex config files or slow config evaluation. Additionally, config was being loaded twice: - **Dev server**: Once in `getStartServerInfo()` just for logging, then again in `getRequestHandlers()` - **Build**: Once for the main build, then again in `getStartServerInfo()` just for logging ### How? **Before:** ``` 1. getStartServerInfo() → loadConfig #1 (slow - blocks on next.config.js) 2. logStartInfo() → logs version, URLs, experimental features 3. "Ready in X" ← time includes config loading 4. getRequestHandlers() → loadConfig #2 ``` **After:** ``` 1. logStartInfo() → logs version, URLs, envInfo (no config needed) 2. "Ready in X" ← reflects actual framework startup time 3. getRequestHandlers() → loadConfig (once) 4. logExperimentalInfo() → logs experimental features ``` **Changes:** - Split `logStartInfo` into two functions: - `logStartInfo()` - basic info (version, URLs, env) - no config needed - `logExperimentalInfo()` - experimental features and cacheComponents - Removed `getStartServerInfo()` which was loading config unnecessarily - Added `experimentalFeatures` and `cacheComponents` to `ServerInitResult` so they can be passed back after config loads - In build, added `reportExperimentalFeatures` callback to the existing `loadConfig` call instead of loading config twice --------- Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
When self-hosting Next.js, it's possible to customize a cache handler for ISR. Self-hosted Next.js also makes use of a similar cache interface for `next/image`, but there's no way to customize this cache handler. This plugs into the [existing ability](https://nextjs.org/docs/app/api-reference/config/next-config-js/incrementalCacheHandlerPath) to customize a `cacheHandler` to support the `IMAGE` type. This lets you implement whatever constraints you might have on your self-hosted environment, eg the need for an LRU cache, or storing optimized images outside of disk. This is currently opt-in because it's a breaking change for cache handlers to start receiving `IMAGE` cache types, which we'll make default in the next major.
Closes PACK-5850 Closes vercel#86132 Closes vercel#86714 The problem was that you get this with scope hoisting: ```js "index.js": () => { // foo.js // exports object read here const self_import = turbopack_import("foo.js") // exports object created here __turbopack_esm__([...], "foo.js") // index.js .... } ``` without scope hoisting, that exports object is already initialized before the module factory runs ```js "foo.js": () => { // exports initialized here const self_import = turbopack_import("foo.js") __turbopack_esm__([....]); } .... ``` so let's fix it by making it hoist the `__turbopack_esm__` statement (just as we did before always) in the case of self-imports --- This bug is actually very similar to vercel#82827. It also executes `data.js` twice. The problem is that if a module imports itself, ```js "[project]/input/data.js [test] (ecmascript)", ((__turbopack_context__) => { "use strict"; var __TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__$3c$locals$3e$__ = __turbopack_context__.i("[project]/input/data.js [test] (ecmascript) <locals>"); var __TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__ = __turbopack_context__.i("[project]/input/data.js [test] (ecmascript)"); __turbopack_context__.s([ "Data", 0, __TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__, "bar", ()=>__TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__$3c$locals$3e$__["bar"], "foo", ()=>__TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__$3c$locals$3e$__["foo"] ]); }), ``` and with scope hoisting, the `import("id")` happens before the `esm_export(..., "id")`: ```js module.exports = [ "[project]/index.js [test] (ecmascript)", ((__turbopack_context__) => { "use strict"; // MERGED MODULE: [project]/index.js [test] (ecmascript) ; // MERGED MODULE: [project]/data.js [test] (ecmascript) <locals> ; function foo() { return 'foo'; } function bar() { return 'bar'; } ; __turbopack_context__.s([ "bar", ()=>bar, "foo", ()=>foo ], "[project]/data.js [test] (ecmascript) <locals>"); // MERGED MODULE: [project]/data.js [test] (ecmascript) <export * as Data> ; // MERGED MODULE: [project]/data.js [test] (ecmascript) ; var __TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__$3c$locals$3e$__ = __turbopack_context__.i("[project]/data.js [test] (ecmascript) <locals>"); var __TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__ = __turbopack_context__.i("[project]/data.js [test] (ecmascript)"); __turbopack_context__.s([ "Data", 0, __TURBOPACK__imported__module__$5b$project$2f$input$2f$data$2e$js__$5b$test$5d$__$28$ecmascript$29$__, "bar", ()=>bar, "foo", ()=>foo ], "[project]/data.js [test] (ecmascript)"); ```
https://doc.rust-lang.org/reference/attributes/derive.html#r-attributes.derive.automatically_derived We discussed this during our morning team meeting. I had an LLM write this. This does not identify any new dead code.
This auto-generated PR updates the development integration test manifest used when testing Rspack.
…ercel#87993) ### What? Adds a Claude Code plugin marketplace to the Next.js repository with an initial `cache-components` plugin that provides expert guidance for Cache Components and Partial Prerendering (PPR). ### Why? Cache Components introduces a new programming model that combines Partial Prerendering, Dynamic I/O, and the `"use cache"` directive. Team members and contributors need guidance on: - The `'use cache'` directive and its variants (`private`, `remote`) - Cache lifetime configuration with `cacheLife()` and predefined profiles - Cache tagging and invalidation with `cacheTag()`, `updateTag()`, `revalidateTag()` - Parameter permutation rendering and subshell generation with `generateStaticParams` - Migration from deprecated `revalidate` and `dynamic` segment configs - Build-time error resolution (dynamic data outside Suspense, uncached data, etc.) A Claude Code plugin allows this knowledge to be distributed directly from the repo and proactively activated when working in projects with `cacheComponents: true`. ### How? Introduces the `.claude-plugin/` directory structure: ``` .claude-plugin/ ├── marketplace.json ← Marketplace catalog (name: "nextjs") └── plugins/ ├── README.md ← Documentation for adding plugins └── cache-components/ ├── .claude-plugin/plugin.json ← Plugin manifest ├── README.md ← Plugin documentation └── skills/cache-components/ ├── SKILL.md ← Core concepts, mental model (496 lines) ├── REFERENCE.md ← API reference, migrations (875 lines) ├── PATTERNS.md ← 12 production patterns (781 lines) └── TROUBLESHOOTING.md ← Build errors, debugging (721 lines) ``` **Installation:** ```bash /plugin marketplace add vercel/next.js /plugin install cache-components@nextjs ``` **Key skill features:** - **Proactive activation** when `cacheComponents: true` is detected in next.config - **Mental model decision tree** for thinking through caching decisions - **Cache scope clarification** explaining what creates new cache entries - **updateTag vs revalidateTag guide** with decision table and examples - **Migration scenarios** from deprecated `revalidate`/`dynamic` segment configs - **Quick debugging checklist** for common cache issues - **Parameter permutation rendering** (how `generateStaticParams` creates subshells) - **12 production patterns** including e-commerce, multi-tenant SaaS, and subshell composition ### Documentation Highlights | Section | Description | |---------|-------------| | Mental Model | Decision tree for when to use `'use cache'`, `'use cache: private'`, or `<Suspense>` | | Cache Scope | What creates new cache entries (function identity, arguments, file path) | | updateTag vs revalidateTag | Decision guide with e-commerce examples | | Migration Scenarios | Before/after examples for `revalidate`, `force-dynamic`, and ISR patterns | | Quick Debugging Checklist | Copy-paste checklists for common issues |
This auto-generated PR updates the production integration test manifest used when testing Rspack.
… response cache (vercel#88509) ## Summary Implements an LRU cache with compound keys for the minimal mode response cache to improve cache hit rates during parallel revalidation scenarios. **Problem**: The previous single-entry cache (`previousCacheItem`) keyed by pathname caused cache collisions when multiple concurrent invocations (e.g., during ISR revalidation) accessed the same pathname. Each invocation would overwrite the previous entry, leading to cache misses and redundant work. **Solution**: An LRU cache using compound keys (`pathname + invocationID`) that allows multiple invocations to cache entries for the same pathname independently: ``` Cache Key Structure ───────────────────── /blog/post-1\0inv-abc → {entry, expiresAt} /blog/post-1\0inv-def → {entry, expiresAt} /blog/post-1\0__ttl__ → {entry, expiresAt} (TTL fallback) /api/data\0inv-ghi → {entry, expiresAt} ``` ### Cache Key Strategy - **With `x-invocation-id` header**: Entries are keyed by invocation ID for exact-match lookups (always a cache hit if the entry exists) - **Without header (TTL fallback)**: Entries use a `__ttl__` sentinel key and validate via expiration timestamp ### Configuration via Environment Variables Cache sizing can be tuned via environment variables (using `NEXT_PRIVATE_*` prefix for infrastructure-level settings): | Environment Variable | Default | Description | |---------------------|---------|-------------| | `NEXT_PRIVATE_RESPONSE_CACHE_MAX_SIZE` | 150 | Max entries in the LRU cache | | `NEXT_PRIVATE_RESPONSE_CACHE_TTL` | 10000 | TTL in ms for cache entries (fallback validation) | ### LRU Cache Enhancement Added an optional `onEvict` callback to `LRUCache` that fires when entries are evicted due to capacity limits. This enables tracking evicted invocation IDs for warning detection without introducing timer-based cleanup. ### Eviction Warnings When a cache entry is evicted and later accessed by the same invocation, a warning is logged suggesting to increase `NEXT_PRIVATE_RESPONSE_CACHE_MAX_SIZE`. This helps developers tune cache sizes for their workload. ### Additional Changes - Renamed header from `x-vercel-id` to `x-invocation-id` for clarity - Added `withInvocationId()` test helper for cache testing ## Test Plan - Existing response cache tests pass with updated header name - Unit tests for `LRUCache` including `onEvict` callback behavior - Updated standalone mode tests to use `withInvocationId()` helper
…rcel#88496) Original [PR](vercel#88012) has a bug that attempted to validate on environment variables. Edited function to not run validations on environment variables, only on the user provided deployment id. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: JJ Kasper <[email protected]>
Add a hydration diff indicator on the top right corner to tell users which line is from client and which line is from server. Heard feedback from users that sometimes they got confused about which line is from where. Now they'll clear know that: - `+ Client` shows `+` is client rendered content - `- Server` shows `+` is server rendered content <img width="579" height="225" alt="image" src="https://github.com/user-attachments/assets/5285709a-044a-4d7f-87f9-08f1199eed4e" />
…88492) ### What? * task type is stored as CachedDataItem * reverse task cache is removed
…88889) ### What? save a few bits
### What? do not persist EcmascriptChunkItemContent It's already stored as part of the module factory
### What? improve module type error message
…ercel#88760) ### What? Allow keys for selective reads to be something equivalent to the key instead of only exactly a reference of the key.
…ercel#80062) ### What? Cache the before plugin matching
Add next.js glossary so we can reference concepts in guides without having to re-explain them. Guides should be succint, like a skill :) Closes: https://linear.app/vercel/issue/DOC-5574/nextjs-glossary --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
### What? Adds a Cursor command file (`.cursor/commands/gt-workflow.md`) that provides guidance on using Graphite (gt) instead of git for branch and commit operations. ### Why? To make the Graphite workflow easily accessible within Cursor IDE via the `/gt-workflow` command, helping contributors follow the repository's git workflow conventions. ### How? Creates a new Markdown command file that includes: - Quick reference table of Graphite commands - Safety rules for stack operations - Multi-branch fix workflow example - Checklist for verification
…ature (vercel#88925) Enhance our debugging output to include information about TaskType values example output: ``` Task type cache stats: 337.82MiB (364.22MiB) 49.98MiB (53.43MiB) <ModuleAssetContext as AssetContext>::resolve_asset x 417238 avg 125B (134B) 29.38MiB (31.46MiB) resolve x 297288 avg 103B (110B) 17.97MiB (22.58MiB) AssetIdent::new_inner x 135310 avg 139B (174B) 15.16MiB (21.86MiB) <ExternalCjsModulesResolvePlugin as AfterResolvePlugin>::after_resolve x 90748 avg 175B (252B) 12.36MiB (13.32MiB) resolve_internal x 138970 avg 93B (100B) 8.44MiB (7.86MiB) ResolveOrigin::resolve_options x 301586 avg 29B (27B) 8.43MiB (9.47MiB) AfterResolvePluginCondition::matches x 87740 avg 100B (113B) 6.70MiB (6.23MiB) <EsmAssetReference as ModuleReference>::resolve_reference x 472105 avg 14B (13B) 6.48MiB (7.81MiB) read_matches x 81046 avg 83B (101B) 6.40MiB (5.95MiB) <EsmAssetReference as ChunkableModuleReference>::chunking_type x 451598 avg 14B (13B) 6.39MiB (5.94MiB) <EsmAssetReference as ChunkableModuleReference>::binding_usage x 450844 avg 14B (13B) 6.03MiB (5.68MiB) <ModuleAssetContext as AssetContext>::process_resolve_result x 184982 avg 34B (32B) 5.99MiB (5.57MiB) EsmAssetReference::get_referenced_asset x 422883 avg 14B (13B) ``` also add summary stats to the 'task cache stats' ``` Task cache stats: 3.23GiB (6.49GiB) 302.72MiB (769.45MiB) <Code as GenerateSourceMap>::generate_source_map = 313.04KiB (308.92KiB) meta 8524 x 37B (37B), 302.42MiB (769.15MiB) data 8524 x 36.33KiB (92.40KiB) 212.75MiB (592.88MiB) module_factory_with_code_generation_issue = 1.59MiB (1.51MiB) meta 45906 x 36B (34B), 211.16MiB (591.37MiB) data 45906 x 4.71KiB (13.19KiB) 207.89MiB (526.37MiB) EcmascriptBuildNodeChunkContent::code = 1.08MiB (1.12MiB) meta 5163 x 219B (228B), 206.81MiB (525.24MiB) data 5163 x 41.02KiB (104.17KiB) 205.21MiB (577.16MiB) EcmascriptChunkItemContent::new = 1.56MiB (1.63MiB) meta 24577 x 66B (69B), 203.65MiB (575.53MiB) data 24577 x 8.48KiB (23.98KiB) 179.00MiB (441.68MiB) EcmascriptBrowserChunkContent::code = 905.40KiB (967.46KiB) meta 2485 x 373B (398B), 178.11MiB (440.74MiB) data 2485 x 73.40KiB (181.62KiB) ``` Using this information i can see that the largest task type is ~365b so we can safely decrease the size of our scratch buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 261 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="examples/reproduction-template/package.json">
<violation number="1" location="examples/reproduction-template/package.json:9">
P2: The reproduction template is documented to use `next@canary` for verification, but this change pins it to a stable release. That makes new reproductions inconsistent with the README guidance and can cause bugs already fixed in canary to be missed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }, | ||
| "dependencies": { | ||
| "next": "canary", | ||
| "next": "15.5.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The reproduction template is documented to use next@canary for verification, but this change pins it to a stable release. That makes new reproductions inconsistent with the README guidance and can cause bugs already fixed in canary to be missed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/reproduction-template/package.json, line 9:
<comment>The reproduction template is documented to use `next@canary` for verification, but this change pins it to a stable release. That makes new reproductions inconsistent with the README guidance and can cause bugs already fixed in canary to be missed.</comment>
<file context>
@@ -6,7 +6,7 @@
},
"dependencies": {
- "next": "canary",
+ "next": "15.5.8",
"react": "19.2.0",
"react-dom": "19.2.0"
</file context>
| "next": "15.5.8", | |
| "next": "canary", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR attempts to (1) expand unit test coverage for several low-level utilities, (2) standardize/pin next dependency versions across the repo (including examples/fixtures), and (3) adjust CI by adding CodeQL scanning and simplifying an existing workflow.
Changes:
- Expanded unit tests for
addBasePath, build logging helpers,getProjectDir/detectTypo, and exported constants. - Pinned/updated
nextversions across many workspaces/examples/fixtures and updatedpnpm-lock.yamlaccordingly. - Added a new CodeQL workflow and removed the
buildPassedjob frombuild_and_deploy.yml; added aTEST_ENHANCEMENTS.mdsummary doc.
Reviewed changes
Copilot reviewed 248 out of 261 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/router-add-base-path.test.ts | Adds extensive coverage for addBasePath behavior under different env configurations. |
| test/unit/build-output-log.test.ts | Adds coverage for warnOnce/errorOnce caching and the various log helpers. |
| test/unit/get-project-dir.test.ts | Adds tests for getProjectDir and typo detection behavior. |
| test/unit/phaseConstants.test.ts | Adds broader assertions for exported constants/enums and conventions. |
| TEST_ENHANCEMENTS.md | Documents the intended test coverage improvements. |
| .github/workflows/codeql.yml | Introduces CodeQL scanning workflow (currently invalid YAML per review comments). |
| .github/workflows/build_and_deploy.yml | Removes the buildPassed job. |
| package.json | Pins root next dependency (currently to a version flagged as vulnerable in lockfile). |
| pnpm-lock.yaml | Updates lockfile for new Next versions; includes a deprecation/security notice for [email protected]. |
| turbopack/packages/turbo-tracing-next-plugin/package.json | Pins next in devDependencies for the tracing plugin. |
| turbopack/packages/turbo-tracing-next-plugin/test/with-mongodb-mongoose/package.json | Pins next in a turbopack test fixture. |
| crates/next-core/js/package.json | Pins next dependency for the next-core JS workspace. |
| apps/docs/package.json | Updates next version used by docs app. |
| apps/bundle-analyzer/package.json | Updates next version used by bundle-analyzer app. |
| packages/third-parties/package.json | Updates canary next version used by the third-parties package. |
| packages/next-codemod/bin/testfixtures/*/package.json | Updates next versions used in codemod CLI fixtures. |
| test/.stats-app/package.json | Pins next in stats app fixture. |
| test/integration/styled-jsx-plugin/app/package.json | Pins next in an integration test fixture. |
| test/e2e/next-test/first-time-setup-{js,ts}/package.json | Pins next in first-time setup e2e fixtures. |
| bench/*/package.json | Pins next versions in benchmark workspaces. |
| examples/**/package.json | Pins next versions across many examples. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe('with basePath set', () => { | ||
| const originalBasePath = process.env.__NEXT_ROUTER_BASEPATH | ||
| const originalManualBasePath = process.env.__NEXT_MANUAL_CLIENT_BASE_PATH | ||
|
|
||
| beforeEach(() => { | ||
| process.env.__NEXT_ROUTER_BASEPATH = '/docs' | ||
| delete process.env.__NEXT_MANUAL_CLIENT_BASE_PATH | ||
| }) |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addBasePath captures __NEXT_ROUTER_BASEPATH at module initialization time (see packages/next/src/client/add-base-path.ts), but this test sets process.env.__NEXT_ROUTER_BASEPATH in beforeEach after importing addBasePath. As a result, the "with basePath set" assertions won't reflect the updated env and will fail. Consider setting env vars before importing the module (e.g., jest.resetModules() + require(...) after setting env) or using jest.isolateModules per suite.
test/unit/get-project-dir.test.ts
Outdated
| it('should resolve provided directory path', () => { | ||
| const { getProjectDir } = require('next/dist/lib/get-project-dir') | ||
| const result = getProjectDir('.', false) | ||
| expect(result).toBeTruthy() | ||
| expect(result).toContain('test') | ||
| }) |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getProjectDir('.', false) resolves to the current working directory (repo root in CI), which typically will not contain the substring 'test'. This expectation is brittle and likely to fail depending on where tests are run. Prefer asserting equality with realpathSync(process.cwd()), or at least that the path exists / is absolute.
test/unit/phaseConstants.test.ts
Outdated
| describe('constant immutability', () => { | ||
| it('should not allow reassignment of phase constants', () => { | ||
| expect(() => { | ||
| // @ts-expect-error - testing immutability | ||
| PHASE_EXPORT = 'something-else' | ||
| }).toThrow() | ||
| }) |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "immutability" test is only verifying that the local imported binding is read-only/const, not that the exported value is immutable. It will throw regardless of the implementation of next/constants, so it doesn't add meaningful coverage. Consider removing it or replacing it with a behavior-based assertion that can actually regress.
.github/workflows/codeql.yml
Outdated
| matrix: | ||
| include: | ||
| - language: actions | ||
| build-mode: none | ||
| - language: javascript-typescript | ||
| build-mode: none | ||
| - language: python | ||
| build-mode: none | ||
| - language: rust | ||
| build-mode: none |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The YAML indentation for the matrix.include list is invalid: the - language: ... entries need to be indented under include:. As written, this workflow won't parse and won't run.
.github/workflows/codeql.yml
Outdated
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The steps: section is not properly indented: step entries (- name: ...) must be nested under steps:. As written, the workflow file is invalid YAML and GitHub Actions will reject it.
| "devDependencies": { | ||
| "next": "workspace:*" | ||
| "next": "15.5.8" |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing this dev dependency from workspace:* to a pinned next version means the plugin's tests/dev setup no longer exercise the in-repo packages/next build. In this monorepo that tends to defeat the purpose of plugin integration testing, and 15.5.8 is also flagged as vulnerable in the lockfile. Consider keeping workspace:* (or otherwise documenting why this package must test against a published Next) and avoid the vulnerable version.
crates/next-core/js/package.json
Outdated
| }, | ||
| "dependencies": { | ||
| "next": "workspace:*" | ||
| "next": "15.5.8" |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching next from workspace:* to a pinned published version makes this workspace stop testing against the in-repo packages/next code and can lead to duplicated/mismatched Next installs. Given the repo uses workspaces heavily, consider keeping workspace:* here (and avoid pinning to 15.5.8, which is marked vulnerable in the lockfile).
| "next": "15.5.8" | |
| "next": "workspace:*" |
test/unit/build-output-log.test.ts
Outdated
| describe('warnOnce caching behavior', () => { | ||
| const original = console.warn | ||
| let messages: any[] | ||
|
|
||
| beforeEach(() => { | ||
| messages = [] | ||
| console.warn = (m: any) => messages.push(m) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| console.warn = original | ||
| }) | ||
|
|
||
| it('should cache based on complete message combination', () => { | ||
| warnOnce('part1', 'part2') | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('part1', 'part2') | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('part1', 'part3') | ||
| expect(messages.length).toBe(2) | ||
| }) | ||
|
|
||
| it('should handle numeric arguments', () => { | ||
| warnOnce('count', 42) | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('count', 42) | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('count', 43) | ||
| expect(messages.length).toBe(2) | ||
| }) | ||
|
|
||
| it('should handle boolean arguments', () => { | ||
| warnOnce('flag', true) | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('flag', false) | ||
| expect(messages.length).toBe(2) | ||
| }) | ||
|
|
||
| it('should handle object arguments', () => { | ||
| warnOnce('object', { key: 'value' }) | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('object', { key: 'value' }) | ||
| expect(messages.length).toBe(1) | ||
| }) | ||
|
|
||
| it('should handle empty string arguments', () => { | ||
| warnOnce('') | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('') | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce('', '') | ||
| expect(messages.length).toBe(2) | ||
| }) | ||
|
|
||
| it('should handle undefined arguments', () => { | ||
| warnOnce(undefined) | ||
| expect(messages.length).toBe(1) | ||
|
|
||
| warnOnce(undefined) | ||
| expect(messages.length).toBe(1) | ||
| }) |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warnOnce keeps a module-level LRU cache keyed by message.join(' '). This cache persists across tests, and values like warnOnce(''), warnOnce(undefined), and warnOnce(null) all produce the same key (''), so later tests can be skipped unexpectedly depending on execution order. To make these tests reliable, isolate/reset the module between tests (e.g., jest.resetModules() in beforeEach and require(...) inside each test) or otherwise clear the cache.
test/unit/get-project-dir.test.ts
Outdated
| beforeAll(() => { | ||
| originalCwd = process.cwd() | ||
| originalProcessEnv = { ...process.env } | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| process.env = originalProcessEnv | ||
| }) |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reassigning process.env (process.env = originalProcessEnv) replaces Node’s special process.env object with a plain object and can break other tests that rely on environment behavior. Instead, restore only the keys you mutate (or snapshot/restore via Object.assign(process.env, ...) plus deleting extra keys).
| "nanoid": "3.1.30", | ||
| "next": "workspace:*", | ||
| "next": "15.5.8", | ||
| "node-fetch": "2.6.7", |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pinning the repo root next dependency to 15.5.8 is risky here: (1) the lockfile marks [email protected] as deprecated due to a security vulnerability, and (2) this repo already uses workspace:* for internal packages (e.g. create-next-app, eslint-config-next), so switching next away from the workspace version can break local development/testing against packages/next. Prefer using workspace:* (or at least a non-vulnerable patched version) consistently.
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: Copilot <[email protected]>
For Contributors
Improving Documentation
pnpm prettier-fixto fix formatting issues before opening the PR.Fixing a bug
This pull request introduces several important changes across the repository, focusing on three main areas: enhanced test coverage, dependency updates (notably for
next), and CI workflow improvements. The most significant update is the addition of a comprehensive test enhancement document summarizing major improvements in test coverage and quality. Additionally, thenextdependency is updated to specific versions across multiple packages and examples for consistency and stability. Finally, a new advanced CodeQL workflow is added for improved code scanning, and a redundant build workflow job is removed.Test Coverage Enhancements:
TEST_ENHANCEMENTS.md, detailing comprehensive improvements to test coverage and quality for key unit tests, including expanded test suites, edge case handling, integration testing, and improved test organization and documentation.Dependency Updates:
nextdependency to specific stable versions (primarily15.5.8or later) in multiplepackage.jsonfiles across apps, benchmarks, crates, and examples to ensure consistency and avoid issues with workspace references or outdated versions. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17]CI/CD and Workflow Improvements:
.github/workflows/codeql.ymlfor advanced CodeQL static analysis, enabling scheduled and PR-based security/code scanning for multiple languages.buildPassedjob from.github/workflows/build_and_deploy.ymlto streamline the CI workflow and eliminate unnecessary steps.issues linked usingfixes #numberAdding a feature
fixes #numberFor Maintainers
What?
Why?
How?
Closes NEXT-
Fixes #
-->